-
Notifications
You must be signed in to change notification settings - Fork 361
chore: fix #412 #622 iOS resolver reference when iOS module not installed #714
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
chore: fix #412 #622 iOS resolver reference when iOS module not installed #714
Conversation
…alled - updated export_unity_package_config.json and split the ExternalDependencyManager/Editor/*/Google.IOSResolver.* into its own definition, adding defineConstraints UNITY_EDITOR and UNITY_IOS
added defineConstraints to AssetConfiguration in export_unity_package.py added unit tests for defineConstraints in export_unity_package_test.py
This looks good, I'll try and get it reviewed :) |
Thanks for sending this, and diving into the export_unity_package script! At a first glance it seems good, but I'll want to spend some time testing it with different install configurations to make sure it doesn't cause any issues, which might take a little bit of time. But I'll try to get to it when I can! |
@a-maurice feel free to push any changes or remake this PR as needed :) |
Is it possible to get this change in some way without building locally from latest source? We have https://github.com/googlesamples/unity-jar-resolver.git?path=upm added as an dependency in unity per instructions, but this I presume just gets the latest release, which currently doesn't seem to contain this fix. Since the files lives inside the package folder using this method, which we very much prefer from having it live in the Assets/ folder, we can't modify served file locally to fix it manually. As such this issue is preventing any build from succeeding for us in Unity 6. |
I disagree. We definitely don't want it assets folder. It properly belongs in packages since it is external. Downloaded as |
That's what I'm saying, English isn't my first language but yes, I want all plugins out of the Asset folder, because we want to disable for example the google play plugin entirely for build targets where it isn't relevant, which is impossible if it's in the Assets folder. |
Then just unzip the .tar packages and do local embed. But tbh the way it is currently setup is IMHO the best and correct way to package this type of asset. That isn't gonna ever change. |
@a-maurice Any news / eta on this? My only option seems to not use firebase at all and go to AWS and such. |
Sorry for the delay in response to this. To follow up, while this approach will generally work at fixing EDM4U's iOS resolver DLL, it inadvertently can cause problems for libraries that (perhaps incorrectly) assume that the iOSResolver dll will always be present, for example, the Firebase Editor dll. On the positive side, I think I might have a solution that works by changing all the direct dependencies to use reflection instead. Obviously this is a bit more fragile, so I still need to test it thoroughly, but initial testing seems good. |
This will remain true. We only switch the compile symbol and exclude
Yes this properly fixes the missing reference in that dll specifically on non-iOS build targets. I've been using a copy of this fix for ~1 year w/o any problems on iOS, Android, macOS, and Windows standalone builds 😅 |
I'm not surprised it works for you, and a lot of the test cases I tried did work. But at least on my mac, Unity 2021 with iOS support installed, I regularly see this pop up as a logged error, basically whenever the DLL reloads:
Admittedly, it isn't a breaking problem, and I can still run/build on the other platforms without issue, so it is technically better, but there isn't a workaround to prevent that log that I could find with this solution, and I'd like to avoid the confusion on that error log if possible. |
Hmm, I'll check out the specific build configuration and double check the work then. What is the PostProcessAllAssets trace being called? |
Here is the full stack I see:
It is very odd, sometimes it won't show up, other times it will. Generally, the reliable way I'm seeing it is: MacOS, latest Unity 2021.3, with iOS support installed, targeting to build for Android, but with the GoogleServices plist for iOS present. Based on the code comments, it tracks that file because of the settings window regardless of selected platform, which is probably what triggers the iOS Resolver invocation into exception. https://github.com/firebase/firebase-unity-sdk/blob/4d3fcaae4783d6036cd13bccffc80de200e1f4e8/editor/app/src/XcodeProjectPatcher.cs#L256 |
export_unity_package_config.json
and separatedGoogle.IOSResolver.*
into its ownPluginImporter
section, addingdefineConstraints
key withUNITY_EDITOR
andUNITY_IOS
elements.export_unity_package_config.json
with standard json lint, 2 spacesdefineConstraints
toAssetConfiguration
inexport_unity_package.py
defineConstraints
inexport_unity_package_test.py
Fixes
Invalid when platform is not set to IOS or module not installed
Valid when platform is set to IOS
CC @a-maurice, @chkuang-g